The end goal here would be to have an FT-based equivalent of the mne.io.read_epochs/evoked/raw_fieldtrip functionality, but ideally with the info-info transmitted as reliably as possible

That would be great! I have a few quick thoughts that make me pause a bit though:

1. This repo is "minimally maintained" at the moment, in the sense that there is nobody with the dedicated time and expertise to make sure things continue working. Along the same (bad) lines, there are barely any regression tests -- I added some minimal ones and a CI framework some months ago so they could be added in principle, but they don't really exist in an impactful way yet. As such, we try not to mess with too much stuff.
2. To me it sounds like you want to mess with a bunch of stuff (in a good way!), but it would be best if we could help guarantee that we don't break current use cases for FieldTrip and BrainStorm (which AFAIK are the major downstream consumers) plus ideally users -- though I'm not too sure how many of them there are.
3. It might be best to start actually by adding some basic test cases that ensure current functionality round-trips well enough for the FieldTrip and BrainStorm use cases. Then some PR like this could much more safely be merged without risking breaking downstream packages.

What do you think? Would you be up for trying (3) in a separate PR then we merge it and come back to this? (Or you could add to this PR if you want, but it would make me a bit less comfortable from a not-breaking-what-we-have-currently standpoint...) Would you be willing to help maintain this module a bit going forward? I'm not saying 100% that these things are required, but if we could go that route it's much safer than merging this with fingers crossed (after pretty lax reviewing) that everything is okay...

cc @britta-wstnr who I mentioned these concerns/ideas to today, feel free to chat with her if you want, we can discuss here more, or you could hop on to office hours a week from today and we could discuss over video (might be easier)!

cc @ftadel

Thanks for your thoughtful reply Eric! Actually, I already started an answer to your comments earlier today, but somehow I forgot to press the 'comment' button, so the first attempt is now lost to posterity.

A bit less well formulated in a second attempt, then. I understand your concerns, and would be game to provide some test functionality. I have already started this (of course) on the FieldTrip end, and so far focussed on not too fine grained unit tests (basically checking if the overall writing/reading works at all, and whether we get out what we expect). We have our own pet datasets to test with, but @britta-wstnr pointed me to your mne-testing-data repo, which I can also look at.

I will have to think a bit about how easy it would be for me to provide a set of tests for the current functionality in a separate PR, because I need some of the proposed changes to the low-level code to get the intermediate/higher level writing/reading up-and-running to begin with.

Would it be convincing enough to you if I were able to show (on all the 'raw' fif-files in mne-testing-data repo) that I can load them into matlab (using FieldTrip, which for the low-level stuff uses the code from mne-matlab) and write them back into a fif-file (using FieldTrip), conserving the time series data (at single precision)? I think that this is functionality that should work before I started tampering with fieldtrip2fiff and the mne code. Anything more fancy than that (e.g. reading evoked data without troubles, writing epoched data) already would require some upgraded code. I could submit this type of test in a separate PR, for your peace of mind, no problem.

I haven't touched any functionality other than pertaining to the reading of raw/epoched/evoked data, or the writing of events, so I am not sure whether I can contribute to any test functions there (yet).

I wouldn't mind taking some responsibility in maintaining this repo, no problem.

Would it be convincing enough to you if I were able to show (on all the 'raw' fif-files in mne-testing-data repo) that I can load them into matlab (using FieldTrip, which for the low-level stuff uses the code from mne-matlab) and write them back into a fif-file (using FieldTrip), conserving the time series data (at single precision)? I think that this is functionality that should work before I started tampering with fieldtrip2fiff and the mne code. Anything more fancy than that (e.g. reading evoked data without troubles, writing epoched data) already would require some upgraded code. I could submit this type of test in a separate PR, for your peace of mind, no problem...

I wouldn't mind taking some responsibility in maintaining this repo, no problem.

At the end of the day I just want to minimize the likelihood of breaking stuff for people, and the time it takes to un-break it when we (inevitably) accidentally do break it. But I also don't want to stand in the way of much overdue progress here. So if it's a lot easier for you to put tests in this PR for what you've added -- rather than starting with a separate PR to add tests for the code we already have -- that's okay with me, too, especially since you've volunteering to fix bugs you introduce :)

We have our own pet datasets to test with, but @britta-wstnr pointed me to your mne-testing-data repo, which I can also look at.

I didn't put in any mechanisms to download/extract/use the testing data. For now maybe just start from these files, which are part of the this git repo:

https://github.com/mne-tools/mne-matlab/tree/master/matlab/test/data

When you need more files than these (maybe that's already the case?) then we can think about integrating mne-testing-data or some other datasets as needed. It would be better not to add them to this repo in the future, though, if possible. Repo bloat becomes a problem with too many binaries added...

In any case, here is what I did for tests in the PR where I added the infrastructure:

https://github.com/mne-tools/mne-matlab/pull/21/files#diff-b7a5e5caea6fb053481c9a7c37b09c175e5d33cc2c63db72c1a02612f27bcc9cR34

You can see I ignored my own advice and added a file to the repo 🤭

I didn't put in any mechanisms to download/extract/use the testing data.

Ah, of course, this is a different repo ... 🤦 so that was bad advice, sorry @schoffelen !

Well, not too bad actually. I have to get used to the (better than in FieldTrip) practice of unit testing, which requires stuff to be more self-contained. For @larsoner 's background: for FieldTrip we always run a series of (extensive) tests overnight on our local file system, where we can exhaustively use a large amount of data. I just installed Moxunit, and will look into this for the current PR. (PS: any advice regarding the distinction between fiff_ and mne_ functions? Our (i.e. FT's) aim has so far been to use fiff_ functions, which appear the more 'original' ones)

(PS: any advice regarding the distinction between fiff_ and mne_ functions? Our (i.e. FT's) aim has so far been to use fiff_ functions, which appear the more 'original' ones)

Yes I think the fiff_ are generally the lower-level ones that have to do with the FIF format and are probably what you want to focus on for I/O round-trip stuff. The mne_ ones appear to be higher-level stuff like finding events in the data, etc.

Hi @larsoner I was wondering whether you would feel inclined to do a quick review of this PR, if not, I'd be happy to merge without further ado. I think I have added a few key tests that evaluate the new (and some of the old) functionality.

Once this is out of the way, I'd like to sync back into FieldTrip, and then create a new PR from a fresh master branch, that incorporates some speed improvements for the reading of one-tag-per-datasample fif-files.